refactor(bugfix): remove manual skill-loading boilerplate#107
Merged
jwm4 merged 2 commits intoambient-code:mainfrom Apr 19, 2026
Merged
refactor(bugfix): remove manual skill-loading boilerplate#107jwm4 merged 2 commits intoambient-code:mainfrom
jwm4 merged 2 commits intoambient-code:mainfrom
Conversation
The bugfix workflow was engineered around a platform bug where the skill tool wasn't working. Skills were loaded via the Read tool with explicit file paths, and every phase skill had dispatch/return blocks telling the agent which file to re-read after completion. Now that the skill tool works, this scaffolding is unnecessary and actively harmful — it encourages the agent to bypass the platform's native skill invocation. Changes across 13 files in workflows/bugfix/: - Replace file paths with skill names in controller and speedrun - Remove "How to Execute" Read tool instructions from orchestrators - Remove dispatch blocks from all 7 phase skills - Remove return-and-re-read instructions from all phase skills - Remove "Always read skill files" rule from controller - Update ambient.json systemPrompt and CLAUDE.md to use skill names The actual phase logic, gating rules, artifact paths, and recommendation logic are all unchanged. ADR: docs/adr/2026-04-14-skill-invocation-cleanup.md Made-with: Cursor
Contributor
WalkthroughThe pull request adds an ADR and updates the bugfix workflow docs to remove manual skill-loading boilerplate: phase skills drop dispatch/re-read instructions; controller and speedrun now reference and run phases by skill name; system prompt and workflow docs updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
Contributor
|
✅ Actions performedReview triggered.
|
In testing, the agent skipped /review and recommended /pr directly after /test, rationalizing that the fix was simple. Add targeted guidance: always recommend /review before /pr — the agent implemented the fix and is not in a position to objectively evaluate its quality. Only the user can decide to skip review. Made-with: Cursor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workflows/bugfix/.claude/skills/controller/SKILL.md`:
- Around line 37-39: The sentence fragment "Can also be invoked at any point
mid-workflow." in the description of the `summary` skill (/summary) should be
rewritten as a full sentence with an explicit subject; update it to something
like "The summary skill can also be invoked at any point mid-workflow." so it
clearly references the `summary` skill and reads as a complete sentence.
- Around line 94-99: The SKILL.md contains a policy conflict: the section that
mandates recommending `/review` before `/pr` (lines around the “Always recommend
`/review` before `/pr`” paragraph) conflicts with a later option that still
presents `/pr` as a direct alternative; update the document so messaging is
consistent by removing or rewording any suggestion that allows skipping review
and instead present `/pr` only as a follow-up action after `/review` completes,
and adjust the alternative list and examples that reference `/pr` (search for
occurrences of `/review` and `/pr` in the SKILL.md block and the paragraph
around lines 94–109) so every path requires `/review` first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f1e318c2-14d6-45f3-a5ab-26e8dfa295c9
📒 Files selected for processing (1)
workflows/bugfix/.claude/skills/controller/SKILL.md
Comment on lines
+37
to
39
| 9. **Summary** (`/summary`) — the `summary` skill | ||
| Scan all artifacts and present a synthesized summary of findings, decisions, | ||
| and status. Can also be invoked at any point mid-workflow. |
Contributor
There was a problem hiding this comment.
Line 39 is a sentence fragment; make it a full sentence.
“Can also be invoked at any point mid-workflow.” should include an explicit subject for clarity.
🧰 Tools
🪛 LanguageTool
[style] ~39-~39: To form a complete sentence, be sure to include a subject.
Context: ... of findings, decisions, and status. Can also be invoked at any point mid-workfl...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workflows/bugfix/.claude/skills/controller/SKILL.md` around lines 37 - 39,
The sentence fragment "Can also be invoked at any point mid-workflow." in the
description of the `summary` skill (/summary) should be rewritten as a full
sentence with an explicit subject; update it to something like "The summary
skill can also be invoked at any point mid-workflow." so it clearly references
the `summary` skill and reads as a complete sentence.
Comment on lines
+94
to
+99
| **Always recommend `/review` before `/pr`.** Do not recommend skipping review, even for | ||
| fixes that seem simple or mechanical. You implemented the fix and wrote the | ||
| tests — you are not in a position to objectively evaluate their quality. | ||
| Review exists precisely to catch what the fixer misses. Only the user can | ||
| decide to skip it. | ||
|
|
Contributor
There was a problem hiding this comment.
Policy conflict: /review is mandatory before /pr, but options still suggest direct /pr.
Line 94-99 forbids recommending skip-review, but Line 109 still offers /pr as an alternative without requiring completed review. This contradiction will produce inconsistent controller behavior.
Suggested patch
@@
Recommended next step: /test — verify the fix with regression tests.
Other options:
- /review — critically evaluate the fix before testing
-- /pr — if you've already tested manually and want to submit
+- /document — if testing is complete and you want to prepare release notes before review🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workflows/bugfix/.claude/skills/controller/SKILL.md` around lines 94 - 99,
The SKILL.md contains a policy conflict: the section that mandates recommending
`/review` before `/pr` (lines around the “Always recommend `/review` before
`/pr`” paragraph) conflicts with a later option that still presents `/pr` as a
direct alternative; update the document so messaging is consistent by removing
or rewording any suggestion that allows skipping review and instead present
`/pr` only as a follow-up action after `/review` completes, and adjust the
alternative list and examples that reference `/pr` (search for occurrences of
`/review` and `/pr` in the SKILL.md block and the paragraph around lines 94–109)
so every path requires `/review` first.
This was referenced Apr 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/skills/*/SKILL.md) with skill names (the assess skill) so the agent uses the platform's native skill invocation instead of the Read toolContext
The bugfix workflow was engineered around a platform bug where the skill tool wasn't working. Skills were loaded via the Read tool with explicit file paths, and every phase skill had dispatch/return blocks telling the agent which file to re-read after completion. Now that the skill tool works, this scaffolding is unnecessary — it encourages the agent to bypass native skill invocation and makes the workflow non-portable across different runners.
ADR:
docs/adr/2026-04-14-skill-invocation-cleanup.mdWhat changed
ambient.jsonCLAUDE.mdcontroller/SKILL.mdspeedrun/SKILL.mdpr/SKILL.mdsummary/SKILL.mdWhat did NOT change
Test plan
ambient.jsonparses correctly/diagnose)Custom Workflow fields for testing
https://github.com/jwm4/workflows.gitrefactor/bugfix-skill-invocation-cleanupworkflows/bugfixMade with Cursor